Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Gauge] Vis editors gauge legacy percent mode. #126318

Merged

Conversation

Kuznietsov
Copy link
Contributor

@Kuznietsov Kuznietsov commented Feb 24, 2022

Summary

Completes part of #124925

In progress...

Screenshot 2022-03-04 at 14 19 24

Screenshot 2022-03-04 at 13 28 01

Screenshot 2022-03-04 at 14 16 45

Screenshot 2022-03-04 at 14 16 54

@Kuznietsov Kuznietsov self-assigned this Feb 24, 2022
@Kuznietsov Kuznietsov added v8.2.0 NeededFor:VisEditors Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Mar 3, 2022
@Kuznietsov
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

merge conflict between base and head

# Conflicts:
#	src/plugins/chart_expressions/expression_gauge/public/components/gauge_component.tsx
#	src/plugins/vis_types/gauge/public/to_ast.ts
#	src/plugins/vis_types/gauge/public/utils/palette.ts
@Kuznietsov Kuznietsov requested a review from crob611 March 4, 2022 09:54
@Kuznietsov Kuznietsov changed the title [WIP][Gauge] Vis editors gauge legacy percent mode. [Gauge] Vis editors gauge legacy percent mode. Mar 4, 2022
@Kuznietsov
Copy link
Contributor Author

@elasticmachine merge upstream

@Kuznietsov Kuznietsov marked this pull request as ready for review March 4, 2022 13:58
@Kuznietsov Kuznietsov requested a review from a team as a code owner March 4, 2022 13:58
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

@Kuznietsov
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula
Copy link
Contributor

@elasticmachine merge upstream

Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on Chrome and works as expected. Code LGTM. Why do we consider it deprecated and don't want to use in Lens? Is it because we can achieve the same results with formula?

@Kuznietsov
Copy link
Contributor Author

Kuznietsov commented Mar 8, 2022

Tested on Chrome and works as expected. Code LGTM. Why do we consider it deprecated and don't want to use in Lens? Is it because we can achieve the same results with formula?

@mbondyra, Thanks for the approval =) what about the deprecated, we are following up the defined pattern at the heatmap and other visualizations. This way of treating the percentage mode is required at the vislib charts, but not used in the Lens and, as I understand, will be removed at some point.

cc @stratoula, WDYT? Have I missed something?

@stratoula
Copy link
Contributor

Yes Yaroslav exactly. The percentage mode in vislib (also used in heatmaps) is considered deprecated. I have also seen users being confused by it. We are not thinking of adding this mode in Lens.

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM. Tested it in Chrome and works fine! Feel free to merge it when CI is green :)

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
expressionGauge 31 32 +1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
expressionGauge 69 70 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
expressionGauge 9.1KB 9.7KB +690.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
expressionGauge 14.5KB 14.8KB +283.0B
visTypeGauge 9.7KB 9.8KB +54.0B
total +337.0B
Unknown metric groups

API count

id before after diff
expressionGauge 69 70 +1

References to deprecated APIs

id before after diff
visTypeGauge 0 1 +1

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @Kunzetsov

@Kuznietsov Kuznietsov merged commit 720fbed into elastic:main Mar 8, 2022
jloleysens added a commit to jloleysens/kibana that referenced this pull request Mar 8, 2022
…ed-unexpectedly-error

* 'main' of github.com:elastic/kibana: (46 commits)
  Fix copy and pasted renderer user_name test (elastic#126663)
  [Gauge] Vis editors gauge legacy percent mode. (elastic#126318)
  Remove all cases related code from timelines (elastic#127003)
  Hide Enterprise search panel when no nodes are present (elastic#127100)
  [Lens] Fixed flakiness on runtime fields' appearance on the list (elastic#126945)
  [Security Solution][Lists] - Add missing privileges callout to exception lists page (elastic#126874)
  [Security Solution][Lists] - Updates exception flyout edit error messages (elastic#126875)
  [Security Solution][Rules] - Remove rule selection for read only users (elastic#126827)
  Fix session cleanup test (elastic#126966)
  [ftr] implement support for accessing ES through CCS (elastic#126547)
  [type-summarizer] always use normalized paths, fix windows compat (elastic#127055)
  Revert "[ci] Configure hourly pipeline for a small spot instance trial (elastic#126824)"
  Revert "[CI] Expand spot instance trial a bit (elastic#126928)"
  [Alerting] Adding functional tests for alerting and actions telemetry (elastic#126528)
  [Telemetry] Check permissions when requesting telemetry (elastic#126238)
  Don't submit empty seed_urls or sitemap_urls when making a partial crawl request (elastic#126972)
  Remove License Requirement for Enterprise Search App Search Meta Engines (elastic#127046)
  [ML] Adding data recognizer module config cache (elastic#126338)
  skip flaky suite (elastic#126027)
  [Reporting] Improve error logging for rescheduled jobs (elastic#126737)
  ...

# Conflicts:
#	x-pack/plugins/reporting/server/core.ts
#	x-pack/plugins/reporting/server/lib/tasks/execute_report.ts
jloleysens added a commit to jloleysens/kibana that referenced this pull request Mar 8, 2022
…re-browser-errors

* 'main' of github.com:elastic/kibana: (46 commits)
  Fix copy and pasted renderer user_name test (elastic#126663)
  [Gauge] Vis editors gauge legacy percent mode. (elastic#126318)
  Remove all cases related code from timelines (elastic#127003)
  Hide Enterprise search panel when no nodes are present (elastic#127100)
  [Lens] Fixed flakiness on runtime fields' appearance on the list (elastic#126945)
  [Security Solution][Lists] - Add missing privileges callout to exception lists page (elastic#126874)
  [Security Solution][Lists] - Updates exception flyout edit error messages (elastic#126875)
  [Security Solution][Rules] - Remove rule selection for read only users (elastic#126827)
  Fix session cleanup test (elastic#126966)
  [ftr] implement support for accessing ES through CCS (elastic#126547)
  [type-summarizer] always use normalized paths, fix windows compat (elastic#127055)
  Revert "[ci] Configure hourly pipeline for a small spot instance trial (elastic#126824)"
  Revert "[CI] Expand spot instance trial a bit (elastic#126928)"
  [Alerting] Adding functional tests for alerting and actions telemetry (elastic#126528)
  [Telemetry] Check permissions when requesting telemetry (elastic#126238)
  Don't submit empty seed_urls or sitemap_urls when making a partial crawl request (elastic#126972)
  Remove License Requirement for Enterprise Search App Search Meta Engines (elastic#127046)
  [ML] Adding data recognizer module config cache (elastic#126338)
  skip flaky suite (elastic#126027)
  [Reporting] Improve error logging for rescheduled jobs (elastic#126737)
  ...

# Conflicts:
#	x-pack/plugins/reporting/server/lib/tasks/execute_report.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Gauge Vis Gauge and goal visualization impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:medium Medium Level of Effort NeededFor:VisEditors release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants